fix(sweep): gate maintenance-sweep workflow ingest by mtime fingerprint#203
Conversation
The 5-minute maintenance sweep re-ran ingestWorkflowsForSession for every active session every cycle with no change detection — unlike the near-real-time startWorkflowPoll, which skips sessions whose workflow artifacts are unchanged (workflowsMaxMtime + a lastSeen map). ingestWorkflowsForSession fully re-parses every workflow journal and every inner agent-*.jsonl from scratch (parseSubagentFile). On a session that accumulates hundreds of workflow runs, each sweep's full re-parse eventually exceeds SWEEP_INTERVAL_MS; the fire-and-forget sweeps overlap and the event loop pegs at ~100% CPU, so the HTTP server stops accepting connections and the dashboard renders a blank page. DASHBOARD_WORKFLOW_POLL_MS=0 does not help — it only disables the poll, not this sweep path. Apply the same cheap mtime fingerprint the poll already uses: keep a per-sweep sweepWorkflowSeen map and skip a session whose workflowsMaxMtime is unchanged since its last ingest. A completing workflow writes its terminal journal, which advances the fingerprint, so the sweep still catches the running→completed transitions it exists to catch; only genuine no-ops are skipped. The identical gate already runs in production via startWorkflowPoll. Add a regression test asserting the skip/re-ingest decision: the fingerprint is stable when nothing changes (skip) and advances when a new artifact appears (re-ingest). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
bug
|
✅ All contributors have signed the CLA. Thank you! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes the periodic workflow ingestion sweep by skipping active sessions whose workflow artifacts have not changed, using an mtime-based fingerprinting mechanism. It also adds a regression test to verify this gating behavior. The feedback identifies two important issues: first, a potential bug where transient ingestion failures could cause a session to be permanently skipped, which can be resolved by removing the session from the tracking map if the ingestion promise rejects; second, unbounded memory growth in the tracking map, which should be mitigated by evicting inactive sessions at the start of each sweep cycle.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ingestWorkflowsForSession(cleanupDb, { id: row.session_id, transcript_path: row.tp }) | ||
| .then((changed) => { |
There was a problem hiding this comment.
Transient Ingestion Failures Cause Permanent Skip
Setting sweepWorkflowSeen.set(row.session_id, mtime) before the asynchronous ingestWorkflowsForSession completes means that if the ingestion fails (e.g., due to a transient database lock or file system error), the session will still be marked as "seen" for that mtime.
On subsequent sweep ticks, the sweep will skip this session because sweepWorkflowSeen.get(row.session_id) === mtime, meaning the failed ingestion will never be retried until a new workflow artifact is written and the mtime advances.
To make this robust against transient failures, we should delete the session from sweepWorkflowSeen if the ingestion promise rejects, allowing it to be retried on the next sweep cycle.
| ingestWorkflowsForSession(cleanupDb, { id: row.session_id, transcript_path: row.tp }) | |
| .then((changed) => { | |
| ingestWorkflowsForSession(cleanupDb, { id: row.session_id, transcript_path: row.tp }) | |
| .catch((err) => { | |
| sweepWorkflowSeen.delete(row.session_id); | |
| throw err; | |
| }) | |
| .then((changed) => { |
| // Catches workflows that complete without a subsequent hook and flips | ||
| // launch-detected "running" rows to "completed" once their journal lands. | ||
| const { ingestWorkflowsForSession } = require("./lib/workflow-ingest"); | ||
| const { ingestWorkflowsForSession, workflowsMaxMtime } = require("./lib/workflow-ingest"); |
There was a problem hiding this comment.
Unbounded Memory Growth in sweepWorkflowSeen
The sweepWorkflowSeen map is declared in the outer scope and persists across all sweep ticks. However, there is currently no mechanism to evict keys when sessions become inactive, completed, or abandoned. Over time, this will lead to unbounded memory growth (a memory leak) as new sessions are created and eventually archived.
To prevent this, we should evict inactive sessions from the map at the start of each sweep cycle.
const { ingestWorkflowsForSession, workflowsMaxMtime } = require("./lib/workflow-ingest");
const activeIds = new Set(active.map((r) => r.session_id));
for (const id of sweepWorkflowSeen.keys()) {
if (!activeIds.has(id)) {
sweepWorkflowSeen.delete(id);
}
}…e sessions Address review feedback on the workflow-ingest gate: - A transient ingest failure previously stuck a session: the fingerprint was recorded before the async ingest, so on rejection the session was skipped until its artifacts changed again. Delete the fingerprint in .catch so the next sweep retries it. - sweepWorkflowSeen never evicted entries for sessions that went inactive, so it grew unbounded over the process lifetime. Prune it to the current active set at the start of each sweep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I have read the CLA Document and I hereby sign the CLA |
github-project-automation
Bot
moved this to Backlog
in Claude Code AI Agents Monitor Project Board
hoangsonww
moved this from Backlog
to In review
in Claude Code AI Agents Monitor Project Board
github-project-automation
Bot
moved this from In review
to Done
in Claude Code AI Agents Monitor Project Board
2 participants
Problem
The 5-minute maintenance sweep in
server/index.js(step 3, added for #167) re-runsingestWorkflowsForSessionfor every active session on every tick, with no change detection. That ingest is not incremental — it re-parses every workflow journal and every inneragent-*.jsonlfor the session from scratch each call (parseSubagentFile, whole-file line-by-line). With many active sessions that each carry workflow history, the sweep repeatedly re-parses hundreds of MB of transcripts it has already ingested, every 5 minutes, saturating the single event loop with redundant work. On a busy self-hosted instance this pushed sustained CPU high enough that HTTP responses stalled and the dashboard rendered a blank page.startWorkflowPollalready avoids exactly this with a cheap mtime fingerprint (workflowsMaxMtime+ alastSeenmap) — it skips sessions whose workflow artifacts are unchanged. The maintenance sweep never got the same gate. (Note:DASHBOARD_WORKFLOW_POLL_MS=0disables only the poll, not this sweep.)Observed: ~1100 workflow
agent-*.jsonlfiles / ~210 MB across many sessions; sustained ~100% CPU on the main thread, accept queue full, localcurl :4820timing out. After gating, steady-state CPU dropped to ~3% and HTTP latency to ~1.5 ms measured across several sweep cycles.Fix
Apply the same fingerprint gate the poll already uses to the sweep's step 3:
sweepWorkflowSeenmap (pruned to the active set each cycle so it can't grow unbounded), andif (mtime === 0 || sweepWorkflowSeen.get(id) === mtime) continue;before the ingest.A completing workflow writes a new terminal journal, which advances
workflowsMaxMtime, so the sweep still catches therunning → completedtransitions it exists to catch — only genuine no-ops are skipped. On ingest failure the fingerprint is dropped so the next sweep retries (relevant because when the poll is disabled the sweep is the only ingester).Scope — what this does and does not address
This removes the redundant re-parse of unchanged sessions, which is the dominant cost when many sessions carry idle/completed workflow history. It deliberately does not touch the ingest itself, so two pre-existing behaviors remain (out of scope here, flagged for follow-up):
ingestWorkflowsForSession— which would also benefit the 12 s poll, since it shares this path.startSessionSync, which guards withrunning/queued).So this is a targeted, low-risk improvement to the sweep, not a rewrite of the ingest pipeline.
Test
Adds a regression test in
server/__tests__/workflow-ingest.test.jsfor the invariant the gate relies on: the fingerprint is stable across calls when nothing changes (→ skip) and advances when a new artifact lands (→ re-ingest). It usesfs.utimesSyncfor a deterministic mtime bump (not wall-clock), and is mutation-checked to be non-vacuous.The sweep body is an anonymous
setIntervalinsideif (require.main === module)with no exported seam, so the test can't drive the wiring itself; it locks down theworkflowsMaxMtime-based decision the gate depends on. Extracting the sweep body into an exported function would let a future test exercise the wiring end-to-end.